-
-
Notifications
You must be signed in to change notification settings - Fork 141
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
c++ fixes #369
c++ fixes #369
Conversation
incorporates: atom/language-c#252 Note that this particular change was modified. `class public virtual : public virtual Example` is invalid I think, but I don't see a good way to prevent that because the detection must be moved into `patterns > include` because textmate is not multiline regex. See also jeff-hykin/better-cpp-syntax#14 (In fact that whole repostory probably has some improvements) I changed it to include the angle brackets because types can have those atom/language-c#263 atom/language-c#311 atom/language-c#368 For tree-sitter, kinda fix the Discord reported issue (In #support M1 Mac C++ Syntax highlighting) Note that I syntax color only the last function name and not the namespace or colon. But that could easily be changed. For future me, the relevant tree-sitter namespace identifier scopes are: `call_expression > qualified_identifier > identifier` and `function_declarator > qualified_identifier > identifier` I don't know anything about template functions so I left that untouched. So this is probably an incomplete fix. Finally, add the `static_assert` operator. It's technically a directive so it'll appear purple, not blue. Again this could easily be changed so don't hesitate about feedback, idk anything about c++
This looks like a fantastic commit and is very much appreciated! Some research will have to be done to see exactly how this behaves, but ideally @mauricioszabo or @pulsar-edit/core could provide some insight into how accurate this all looks. But otherwise thanks a ton for your contribution! |
@icecream17 do you mind including a code that this fix applies to? |
// class declaration and inheritance on multiple lines (non-tree-sitter)
class A :
public B,
public C,
public D<GenericType>
{
// static_assert in both non-tree-sitter and tree-sitter
static_assert(true);
public: void print();
};
// compare with
class E : public F, public G, public H<GenericType> {}
// virtual public vs public virtual (both should work)
class I : virtual public J {};
class K : public virtual L {};
// "namespaced" function (idk what this is called)
void A::print()
{
// (tree-sitter) `<` and `>` are now properly selected
bool unused = ( 1 < 2 ) && ( 4 > 3 );
}
// "nested namespace definition" c++17 (see https://en.cppreference.com/w/cpp/language/namespace#Namespaces)
namespace abc::def::ghi {} Note that github uses textmate so some of these don't syntax highlight in github too |
Good to see modernization of the language support! And glad to see @mauricioszabo's approval, and the CI results are within current expectations. I'm going to go ahead and merge this one! Thanks for this @icecream17! |
Oh, yeah... If anyone would be interested in writing test cases for this, it would be 5,000 and a half % awesome. Didn't want to require it for merging, and yet, it would make a fantastic follow-up PR to this. |
Description of change:
incorporates:
:fix: #245 #169 #170 atom/language-c#252
I changed it to include the angle brackets because classes can have those (generics).
Fixes #260, adding C++17 nested namespace decls. atom/language-c#263
add static_assert, split operators atom/language-c#311
🐛 Fix scope selectors for angle brackets atom/language-c#368
Alternate Designs
Note for the first incorporation, I only syntax color the last function name and not the namespace or
::
. But that could easily be changed.For future me, the relevant tree-sitter namespace identifier scopes are:
call_expression > qualified_identifier > identifier
andfunction_declarator > qualified_identifier > identifier
I don't know anything about template functions so I left that untouched. So this is probably an incomplete fix.
Finally, add the
static_assert
operator. It's technically a directive so it'll appear purple, not blue. Again this could easily be changed so don't hesitate about feedback, idk anything about c++Verification Process
Will download and check to make sure it works
Applicable issues
Relevant to atom/language-c#246, atom/language-c#245 (pretty much a fix except drawback),
(I split these out bc github doesn't detect that "fixes" applies to multiple issues)
Fixes atom/language-c#169
Fixes atom/language-c#170
Fixes atom/language-c#260
Fixes microsoft/vscode-cpptools#1477
For tree-sitter, kinda fix the Discord reported issue (In #support M1 Mac C++ Syntax highlighting)